-
Notifications
You must be signed in to change notification settings - Fork 181
chore(ci): upgrade checkout to v5 #881
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAll GitHub Actions workflows and a composite action update the checkout step from actions/checkout@v4 to actions/checkout@v5. No other steps, logic, or configurations are modified. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (6)
.github/workflows/app.yml (2)
48-48: Keep a single checkout in the build job; remove duplicates to save minutesThe build job currently performs multiple checkouts (here and again at Line 56), and the composite action also does a checkout. This adds unnecessary time and network I/O.
Apply this diff to remove the redundant second checkout (keep the first one and let the composite action rely on it after removing its internal checkout, see the composite action comment):
- - uses: actions/checkout@v5
56-56: Avoid back-to-back redundant checkoutsRunning checkout twice in the same job (and again inside the composite action) is wasteful. Prefer a single checkout per job and have the composite action assume the repo is already present.
Apply this diff to remove this checkout:
- - uses: actions/checkout@v5.github/actions/mobile-setup/action.yml (1)
38-38: Remove checkout from the composite action to avoid unexpected side-effectsComposites performing
actions/checkoutcause double/triple checkouts in callers (as seen in app.yml), slowing jobs and surprising consumers. Let workflows own checkout; composites should assume the repo is already present.Apply this diff to drop the checkout step from the composite:
- - name: Checkout repository - uses: actions/checkout@v5Follow-up: After removing this, keep a single checkout near the top of each workflow job that uses this composite (e.g., app.yml build job keeps the first checkout and removes subsequent ones).
.github/workflows/contracts.yml (1)
29-29: Remove redundant second checkout to avoid wasted time and side effectsTwo consecutive checkouts in the same job provide no benefit and add a few seconds per run. It can also reset any repo-level state configured by preceding steps.
Apply this diff to remove the duplicate step:
- - uses: actions/checkout@v5.github/workflows/npm-publish.yml (1)
21-21: Pin actions to a commit SHA for supply-chain hardening (optional but recommended)Consider pinning actions/checkout@v5 to the v5.0.0 commit SHA to prevent supply-chain drift.
Would you like me to fetch and propose the exact SHA for actions/checkout v5.0.0 across all workflows in a follow-up PR?
.github/workflows/mobile-deploy.yml (1)
57-57: Consider pinning checkout to a commit SHAOptional hardening: pin actions/checkout@v5 to the release commit SHA.
If you want, I can open a follow-up to pin action SHAs repo-wide.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/actions/mobile-setup/action.yml(1 hunks).github/workflows/app.yml(3 hunks).github/workflows/artifacts.yml(1 hunks).github/workflows/circuits.yml(1 hunks).github/workflows/contracts.yml(1 hunks).github/workflows/general-checks.yml(3 hunks).github/workflows/mobile-deploy.yml(2 hunks).github/workflows/npm-publish.yml(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-14T09:03:08.292Z
Learnt from: aaronmgdr
PR: selfxyz/self#763
File: app/.github/workflows/test-coverage.yml:0-0
Timestamp: 2025-07-14T09:03:08.292Z
Learning: Node.js 22 is supported by actions/setup-nodev4 and has been in Active LTS since October 2024. It works on Linux, macOS, and Windows runners (Windows issues were resolved after version 22.8.0).
Applied to files:
.github/workflows/contracts.yml.github/workflows/npm-publish.yml
🔇 Additional comments (14)
.github/workflows/app.yml (2)
22-22: Upgrade to actions/checkout@v5 looks goodNo functional changes introduced; aligns with the platform requirement.
35-35: Upgrade to actions/checkout@v5 looks goodNo functional changes introduced; aligns with the platform requirement.
.github/workflows/circuits.yml (1)
25-25: Upgrade to actions/checkout@v5 looks goodMatches the repo-wide migration; no behavior change expected.
.github/workflows/general-checks.yml (3)
9-9: Upgrade to actions/checkout@v5 looks goodConsistent with the PR objective; no functional differences.
18-18: Upgrade to actions/checkout@v5 looks goodConsistent with the PR objective; no functional differences.
27-27: Upgrade to actions/checkout@v5 looks goodConsistent with the PR objective; no functional differences.
.github/workflows/artifacts.yml (1)
23-23: Verify self-hosted runner compatibility with actions/checkout@v5Please ensure your self-hosted GitHub Actions runners meet the minimum version requirement and Node.js runtime compatibility:
- File: .github/workflows/artifacts.yml, line 23 (
uses: actions/checkout@v5)- Confirm runners are v2.327.1 or newer (per https://github.com/actions/checkout/releases/tag/v5.0.0)
- Verify the Node.js version installed on those runners matches your workflow’s needs (e.g., Node 16+)
.github/workflows/contracts.yml (1)
23-23: Checkout v5 upgrade looks goodThis aligns with GH runners moving to Node 24 and v5 requirement. No functional risk.
.github/workflows/npm-publish.yml (4)
21-23: Checkout v5 with fetch-depth: 2 remains correct for HEAD^ diffsThis keeps the detect-changes logic working with git diff HEAD^ HEAD. Looks good.
50-50: Checkout v5 upgrade is correct in publish-coreNo functional changes; safe update.
80-80: Checkout v5 upgrade is correct in publish-qrcodeNo functional changes; safe update.
110-110: Checkout v5 upgrade is correct in publish-commonNo functional changes; safe update.
.github/workflows/mobile-deploy.yml (2)
57-57: Checkout v5 upgrade is correct for iOS build jobThis is a straight version bump, no functional change. Good to go.
370-370: Checkout v5 upgrade is correct for Android build jobChange is scoped and safe.
| node-version: '18' | ||
| registry-url: 'https://registry.npmjs.org' | ||
| - uses: actions/checkout@v4 | ||
| - uses: actions/checkout@v5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Remove duplicate checkout within the same job
This second checkout is redundant and can reset working directory state (e.g., any git config or files modified/created by prior steps), potentially causing flaky behavior.
Apply this diff to remove it:
- - uses: actions/checkout@v5📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: actions/checkout@v5 |
🤖 Prompt for AI Agents
.github/workflows/npm-publish.yml around line 116: there is a duplicate "uses:
actions/checkout@v5" within the same job which can reset working-directory
state; remove this redundant checkout step (or delete the single line/step
containing that second checkout) so only the initial checkout remains, and if
the later step relied on specific checkout options, merge those options into the
primary checkout instead.
|
@rejected-l thanks for opening this pull request, appreciate the help. could you please re-open another pull request on the |
|
Sure |
GitHub-hosted runners now use Node 24, so actions/checkout@v5 is required. Minimum runner version v2.327.1. Workflows only updated—no functional changes.
See: https://github.com/actions/checkout/releases/tag/v5.0.0
Summary by CodeRabbit